-
Notifications
You must be signed in to change notification settings - Fork 341
Update futures-timer to 3.0.2 #739
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I didn't check before creating a PR. Another PR for this is already open #738 |
fn next_interval(prev: Instant, now: Instant, interval: Duration) -> Instant { | ||
let new = prev + interval; | ||
if new > now { | ||
return new; | ||
} | ||
|
||
let spent_ns = duration_to_nanos(now.duration_since(prev)).expect("interval should be expired"); | ||
let interval_ns = | ||
duration_to_nanos(interval).expect("interval is less that 427 thousand years"); | ||
let mult = spent_ns / interval_ns + 1; | ||
assert!( | ||
mult < (1 << 32), | ||
"can't skip more than 4 billion intervals of {:?} \ | ||
(trying to skip {})", | ||
interval, | ||
mult | ||
); | ||
prev + interval * (mult as u32) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this logic, but is it safe to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The logic is quite complex and there is not much documentation for next_interval
function. But, as the name suggests, it is trying to find the next Instant
to update the Delay
with. As the new version of Delay
already takes Duration
as argument, we don't have to go through that complex logic to find the next Instant
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
No description provided.